Skip to content

UIEXT-3570: Add examples framework#203

Open
PaulBrnrther wants to merge 2 commits intomasterfrom
todo/UIEXT-3570-lay-out-document-by-example-framework
Open

UIEXT-3570: Add examples framework#203
PaulBrnrther wants to merge 2 commits intomasterfrom
todo/UIEXT-3570-lay-out-document-by-example-framework

Conversation

@PaulBrnrther
Copy link
Copy Markdown
Contributor

UIEXT-3570 (Lay out document-by-example framework)

Copilot AI review requested due to automatic review settings April 16, 2026 15:05
@PaulBrnrther PaulBrnrther requested a review from a team as a code owner April 16, 2026 15:05
@PaulBrnrther PaulBrnrther changed the title UIEXT-3570: WIP UIEXT-3570: Add examples framework Apr 16, 2026
@PaulBrnrther PaulBrnrther force-pushed the todo/UIEXT-3570-lay-out-document-by-example-framework branch from d07403f to 03b9ea2 Compare April 16, 2026 15:06
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Introduces a hidden “Node Dialog Examples” node and supporting types to lay out a document-by-example framework for KNIME node dialogs, starting with an Array Widget example.

Changes:

  • Added an ExampleNodeParameters marker interface plus @NodeParametersExample annotation for example parameter classes.
  • Added a new hidden node (DefaultNodeDialogExamplesNodeFactory + parameters) that lets users select and render example parameter dialogs dynamically.
  • Updated ArrayWidgetExample to act as the first selectable example and enhanced it with additional @Widget metadata.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
org.knime.core.ui/src/eclipse/org/knime/node/parameters/examples/impl/ExampleNodeParameters.java Introduces example-parameters interface + annotation used by the examples framework.
org.knime.core.ui/src/eclipse/org/knime/node/parameters/examples/impl/DefaultNodeDialogExamplesNodeParameters.java Adds selection + dynamic parameter provision for switching between example dialogs.
org.knime.core.ui/src/eclipse/org/knime/node/parameters/examples/impl/DefaultNodeDialogExamplesNodeFactory.java Registers a hidden “Node Dialog Examples” node built via DefaultNode.
org.knime.core.ui/src/eclipse/org/knime/node/parameters/examples/ArrayWidgetExample.java Makes the array widget example discoverable/selectable and improves widget metadata.
org.knime.core.ui/plugin.xml Repoints hidden node registration to the new examples node factory.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +60 to +61
* Every example node parameters class should implement this interface. It will automatically be picked up by the
* {@link DefaultNodeDialogExamplesNodeFactory} and shown in its node dialog.
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Javadoc claims that implementations are “automatically … picked up by the DefaultNodeDialogExamplesNodeFactory”, but the current implementation relies on a hard-coded EXAMPLE_CLASSES list. This is misleading for API consumers; either adjust the doc to describe the manual registration mechanism, or implement actual discovery/registration so the statement is true.

Suggested change
* Every example node parameters class should implement this interface. It will automatically be picked up by the
* {@link DefaultNodeDialogExamplesNodeFactory} and shown in its node dialog.
* Every example node parameters class should implement this interface. Implementations are used by the
* {@link DefaultNodeDialogExamplesNodeFactory} and shown in its node dialog when they are registered with that
* factory.

Copilot uses AI. Check for mistakes.
@ChoicesProvider(ExampleChoicesProvider.class)
@ValueReference(SelectedExampleRef.class)
@Migrate(loadDefaultIfAbsent = true)
String m_selectedExample = EXAMPLE_CLASSES.get(0).getName();
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m_selectedExample is initialized via EXAMPLE_CLASSES.get(0) at field initialization time. If EXAMPLE_CLASSES is ever empty (e.g., during refactoring or feature flags), this will throw during class initialization before the later isEmpty() guard can run. Consider defaulting to an empty string/null and letting computeParameters handle the fallback, or enforcing non-empty via an explicit static validation with a clear error message.

Suggested change
String m_selectedExample = EXAMPLE_CLASSES.get(0).getName();
String m_selectedExample = "";

Copilot uses AI. Check for mistakes.
Comment on lines +94 to +97
return EXAMPLE_CLASSES.stream().map(cls -> {
final var annotation = cls.getAnnotation(NodeParametersExample.class);
return new StringChoice(cls.getName(), annotation.name());
}).toList();
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

computeState assumes every class in EXAMPLE_CLASSES has a @NodeParametersExample annotation (annotation.name()), which will NPE if a class is added without the annotation. Also, the stored identifier uses cls.getName(); if a class later uses @OriginalClassName (supported by DefaultClassIdStrategy), the choice values should come from the same class-id strategy (toIdentifier) to stay consistent.

Copilot uses AI. Check for mistakes.
Comment on lines +121 to +125
final var selected = m_selectedExampleSupplier.get();
if (selected == null || selected.isEmpty()) {
return NodeParametersUtil.createSettings(EXAMPLE_CLASSES.get(0));
}
return NodeParametersUtil.createSettings(getClassIdStrategy().fromIdentifier(selected));
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

computeParameters calls getClassIdStrategy().fromIdentifier(selected) which throws IllegalArgumentException when the persisted m_selectedExample is no longer registered (e.g., class removed/renamed or settings copied between installs). Catch this and fall back to a default example (and/or reset the selection) to avoid failing dialog creation on stale settings.

Suggested change
final var selected = m_selectedExampleSupplier.get();
if (selected == null || selected.isEmpty()) {
return NodeParametersUtil.createSettings(EXAMPLE_CLASSES.get(0));
}
return NodeParametersUtil.createSettings(getClassIdStrategy().fromIdentifier(selected));
final var defaultExampleClass = EXAMPLE_CLASSES.get(0);
final var selected = m_selectedExampleSupplier.get();
if (selected == null || selected.isEmpty()) {
return NodeParametersUtil.createSettings(defaultExampleClass);
}
try {
return NodeParametersUtil.createSettings(getClassIdStrategy().fromIdentifier(selected));
} catch (final IllegalArgumentException ex) {
return NodeParametersUtil.createSettings(defaultExampleClass);
}

Copilot uses AI. Check for mistakes.
Comment on lines 23 to 26
<node
hidden="true"
factory-class="org.knime.core.webui.node.dialog.defaultdialog.hiddenfeaturesnode.HiddenFeaturesNodeFactory">
factory-class="org.knime.node.parameters.examples.impl.DefaultNodeDialogExamplesNodeFactory">
</node>
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change replaces the registered hidden node factory (HiddenFeaturesNodeFactory) with DefaultNodeDialogExamplesNodeFactory, effectively removing the existing “hidden features” node from the repository extension. If the old node is still needed for QA/UX workflows, it should remain registered (e.g., as an additional hidden node) or the PR description should explicitly call out the removal/migration plan.

Copilot uses AI. Check for mistakes.
UIEXT-3570 (Lay out document-by-example framework)

UIEXT-3570: Make it a WebUINodeModel to make it appear

UIEXT-3570 (Lay out document-by-example framework)

UIEXT-3570: Add search tags

UIEXT-3570 (Lay out document-by-example framework)
@PaulBrnrther PaulBrnrther force-pushed the todo/UIEXT-3570-lay-out-document-by-example-framework branch from 3e5640c to 852b35a Compare April 16, 2026 15:34
UIEXT-3570 (Lay out document-by-example framework)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants